HADOOP-15566. Support Opentracing#1846
Closed
smengcl wants to merge 6 commits intoapache:HADOOP-15566-OpenTracingfrom
Closed
HADOOP-15566. Support Opentracing#1846smengcl wants to merge 6 commits intoapache:HADOOP-15566-OpenTracingfrom
smengcl wants to merge 6 commits intoapache:HADOOP-15566-OpenTracingfrom
Conversation
|
💔 -1 overall
This message was automatically generated. |
Contributor
Author
|
HADOOP-16824 is blocking this PR. |
|
💔 -1 overall
This message was automatically generated. |
jojochuang
reviewed
Feb 18, 2020
hadoop-common-project/hadoop-common/src/main/proto/RpcHeader.proto
Outdated
Show resolved
Hide resolved
jojochuang
reviewed
Feb 18, 2020
hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/datatransfer.proto
Outdated
Show resolved
Hide resolved
jojochuang
reviewed
Feb 18, 2020
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
jojochuang
reviewed
Feb 22, 2020
Contributor
There was a problem hiding this comment.
If this method is no longer needed, let's remove it entirely.
Contributor
There was a problem hiding this comment.
did you forget to log variable byteString?
Contributor
There was a problem hiding this comment.
missing a parameter here?
Contributor
There was a problem hiding this comment.
i need to check if the concept matches but io.opentracing.Tracer offers a addReference() API to represent the casual relationship.
adamantal
reviewed
Feb 24, 2020
Contributor
adamantal
left a comment
adamantal
left a comment
There was a problem hiding this comment.
I took a quick glance at this patch. I don't really have context on this, so I don't want to interfere.
I suggested to add javadoc which would describe the new classes and their intended behaviours.
Also, I see the the current TestTracing is empty. Can we try some TDD-approach / add test case to have more insight what is the goal here?
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/tracing/Span.java
Outdated
Show resolved
Hide resolved
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/tracing/TraceScope.java
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Could you please add javadoc here?
jojochuang
reviewed
Mar 4, 2020
Contributor
There was a problem hiding this comment.
This is dangerous
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/ObjectInputStream.html
"Warning: Deserialization of untrusted data is inherently dangerous and should be avoided."
We need an alternative.
Contributor
There was a problem hiding this comment.
Moreover, ObjectInputStream doesn't guarantee compatibility between a server and a client if they are on different version. Finally, I bet it is not optimal serialization format. We should find one that's as short as possible.
Contributor
Author
There was a problem hiding this comment.
Good find @jojochuang . I agree we should definitely use a more optimized solution for serialization/deserialization if possible.
Change-Id: Ia6adf57210161aea6dd4111adee460a8cfe87039
…rdparty.protobuf. nice. Change-Id: I104fbf4fa16f38eb0de63c62e5f2b0a3c96e1fad
Change-Id: I80a113075938defd0902753a331f2adc6289a8f5
…ceInfoProto, BaseHeaderProto/ReleaseShortCircuitAccessRequestProto/ShortCircuitShmRequestProto to DataTransferTraceInfoProto. Change-Id: I53223547601f7fc0f450c3c512728324ef67b3e8
Change-Id: I236788b52f0542d1fca5f902131a815bdfa298f7
Change-Id: I3e622e7febd8d385d5c6fff8c8572accb39c0fc9
Contributor
Author
|
Rebased onto latest trunk (44afe11): Resolved a minor import conflict in NameNode class due to the introduction of |
Contributor
|
I created a branch "HADOOP-15566-OpenTracing" please raise a PR against that branch in the future. Thanks! |
Contributor
Author
I've updated this PR's base branch to |
|
💔 -1 overall
This message was automatically generated. |
This was referenced Jan 20, 2021
Contributor
|
We're closing this stale PR because it has been open for 100 days with no activity. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replacing HTrace with OpenTracing.
mvn install -Pdist -DskipTests -e -Dmaven.javadoc.skip=true -Denforcer.skip=true -DskipShade